-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simple header search and remove dependency to io::Seek #18
base: master
Are you sure you want to change the base?
Simple header search and remove dependency to io::Seek #18
Conversation
I agree that I don't strongly think that #12 is a bug that has to be handled right away, but something that improves the usefulness. Still, I don't think it's a good idea to enforce |
If it's done under the hood, it's done unconditionally, even though it adds a non zero overhead. So it's a bad idea. |
Wait disregard that, all of |
So I don't have to care about async for this implementation? If so, what are other problems left? |
...instead of returning an error.
My previous code returns Error when failed to find a header, which is different from what is written in the document, so I corrected. As an side effect, the code does no longer use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Hmm thinking about this some more, it basically adds the requirement to copy data once more if you want to retain performance, which isn't optimal for performance due to the additional copy/buffering. Avoiding this was a deliberate design choice. See my comment above. Maybe the new behaviour could be exposed under a different API. Then users can choose themselves which approach they want (lots of Read calls or occasional seeking). |
closes #12
I simplified the algorithm of finding the page header. This method requires a lot of
read()
function called, but the performance degression can be avoided by wrapping the underlying reader withBufReader
or some other buffering reader.The algorithm of searching header is intended to be used only for continued sequence of pages. I am going to introduce another algorithm dedicated for page searching after seek later.